Add OpenVMM virtio device support - #4621

lisa/tools/openvmm.py:156

• Error message doesn’t list the supported network device values, which makes it harder to fix invalid configurations. Consider including the supported values in the exception message (consistent with the schema validation errors).

            raise LisaException(
                f"Unsupported OpenVMM network device: {config.network_device}"
            )

lisa/tools/openvmm.py:162

• Error message doesn’t list the supported IOMMU values, which makes it harder to fix invalid configurations. Consider including the supported values in the exception message (consistent with the schema validation errors).

            raise LisaException(f"Unsupported OpenVMM IOMMU: {config.iommu}")

lisa/tools/openvmm.py:149

• Error message doesn’t list the supported disk device values, which makes it harder to fix invalid configurations. Consider including the supported values in the exception message (consistent with the schema validation errors).

This issue also appears in the following locations of the same file:

• line 154
• line 162

            raise LisaException(
                f"Unsupported OpenVMM disk device: {config.disk_device}"
            )

lisa/sut_orchestrator/openvmm/schema.py:113

• This PR introduces shared TAP subnet/resource-sharing behavior (shared_subnet) in addition to the virtio device selection described in the PR title/description. Please update the PR description to explicitly cover this new behavior (or split it into a separate PR) so reviewers can evaluate scope and risk appropriately.

class OpenVmmNetworkSchema:
    mode: str = OPENVMM_NETWORK_MODE_USER
    device: str = OPENVMM_NETWORK_DEVICE_SYNTHETIC
    shared_subnet: bool = False
    connection_mode: str = OPENVMM_CONNECTION_MODE_FORWARDED_PORT

selftests/test_openvmm_node.py:177

• With schema defaults, queue_count is omitted (None) unless explicitly configured. This assertion should match the default behavior for synthetic networking.

        self.assertEqual(1, launch_config.network_queue_count)

lisa/sut_orchestrator/openvmm/node.py:2133

• When shared_subnet setup fails after iptables rules/dnsmasq state is created but before it’s copied into SharedTapNetworkContext, _teardown_tap_network skips cleanup because shared_tap_network_key is set. This can leak host iptables rules and/or a dnsmasq process on setup errors.

Consider making teardown remove per-node iptables rules/dnsmasq when the shared context hasn’t yet recorded them (e.g., shared_context.input_rules_added / shared_context.dnsmasq_pid_file are empty).

        if not shared_network_key and node_context.tap_input_rules_added:
            rules_to_remove = node_context.tap_input_rules_added
            for rule in rules_to_remove:
                self.host_node.execute(
                    f"iptables -D {rule} || true",

selftests/test_openvmm_node.py:145

• This test configures a synthetic NIC but sets queue_count=1. In the real schema, queue_count defaults to None, and forcing a default of 1 here makes the test validate a behavior that users won’t actually get by default.

This issue also appears on line 177 of the same file.

                    queue_count=1,

lisa/sut_orchestrator/openvmm/node.py:755

• Major: vps_per_socket defaults to processor_count when unspecified, which forces --vps-per-socket to always be passed (changing the default OpenVMM command line for existing runbooks). If the intent is to keep existing behavior unless the runbook explicitly opts in, pass through runbook.vps_per_socket directly and let it remain None by default.

            ),
            processors=processor_count,
            vps_per_socket=(
                runbook.vps_per_socket
                if runbook.vps_per_socket is not None
                else processor_count
            ),
            smt=runbook.smt,

lisa/sut_orchestrator/openvmm/node.py:421

• Major: In create_effective_network(), the shared_subnet branch returns before validate_tap_interface_names() is called. Since tap_name is still suffixed per guest, shared_subnet can silently produce invalid interface names (e.g., exceeding max length or using invalid characters) without the usual validation/error context.

        base_tap_host_cidr = effective_network.tap_host_cidr
        effective_network.tap_name = _increment_name_suffix(
            effective_network.tap_name, guest_index
        )
        if effective_network.shared_subnet:
            if effective_network.address_mode == OPENVMM_ADDRESS_MODE_STATIC:
                effective_network.guest_address = _increment_ip_address(
                    effective_network.guest_address,
                    effective_network.tap_host_cidr,
                    guest_index,
                )
            if effective_network.forward_ssh_port:
                effective_network.forwarded_port += guest_index
                if effective_network.forwarded_port > 65535:
                    raise LisaException(
                        "cannot derive OpenVMM forwarded SSH port from "
                        f"'{network.forwarded_port}' for guest index {guest_index}: "
                        "derived port exceeds 65535. Use a lower base forwarded_port."
                    )
            return effective_network

lisa/tools/openvmm.py:168

• Major: _validate_processor_topology() doesn't validate the relationship between processors and vps_per_socket. As written, vps_per_socket can be greater than processors or not divide evenly into processors, which is likely to produce an invalid or surprising CPU topology at runtime.

    def _validate_processor_topology(self, config: OpenVmmLaunchConfig) -> None:
        if config.vps_per_socket is not None and config.vps_per_socket < 1:
            raise LisaException(
                "OpenVMM vps_per_socket must be at least 1. "
                "Set it to the number of virtual processors in each socket."
            )
        if config.smt and config.smt not in [
            OPENVMM_SMT_AUTO,
            OPENVMM_SMT_FORCE,
            OPENVMM_SMT_OFF,
        ]:
            raise LisaException(
                f"OpenVMM SMT mode '{config.smt}' is not supported. "
                f"Use {OPENVMM_SMT_AUTO}, {OPENVMM_SMT_FORCE}, or "
                f"{OPENVMM_SMT_OFF}."
            )

lisa/sut_orchestrator/openvmm/schema.py:369

• Major: OpenVmmGuestNodeSchema.__post_init__ rejects an empty SMT value. If smt is intended to be optional (empty meaning "use OpenVMM default"), gate the validation on self.smt being non-empty (mirrors OpenVmmLaunchConfig validation).

        if self.smt not in [OPENVMM_SMT_AUTO, OPENVMM_SMT_FORCE, OPENVMM_SMT_OFF]:

lisa/sut_orchestrator/openvmm/schema.py:317

• Major: OpenVmmGuestNodeSchema.smt defaults to OPENVMM_SMT_OFF, which means OpenVMM launches will always emit --smt off even when the runbook doesn't specify SMT. To preserve existing OpenVMM defaults unless explicitly configured, make the schema default empty and rely on the tool layer to validate non-empty values.

This issue also appears on line 369 of the same file.

    smt: str = OPENVMM_SMT_OFF

lisa/sut_orchestrator/openvmm/node.py:754

• Major: launch() always forces vps_per_socket to processor_count when the runbook value is unset, which makes the generated OpenVMM command always include --vps-per-socket and changes default behavior. If the intent is to preserve existing defaults unless explicitly configured, pass through runbook.vps_per_socket as-is (None) and let OpenVmmLaunchConfig omit the flag.

            vps_per_socket=(
                runbook.vps_per_socket
                if runbook.vps_per_socket is not None
                else processor_count
            ),

lisa/sut_orchestrator/openvmm/schema.py:122

• Minor: This PR adds shared_subnet support and shared TAP resource management, but the PR title/description focuses on virtio device selection. Please update the PR description (and/or title) to mention the shared-subnet feature and runner behavior change so reviewers understand the full scope and can validate appropriately.

    shared_subnet: bool = False

lisa/sut_orchestrator/openvmm/schema.py:369

• If smt is allowed to be unset/empty to preserve prior behavior, the schema validation should treat an empty value as “not specified” (similar to lisa.tools.openvmm.OpenVmm._validate_processor_topology). As written, an empty string would fail validation.

        if self.smt not in [OPENVMM_SMT_AUTO, OPENVMM_SMT_FORCE, OPENVMM_SMT_OFF]:

lisa/sut_orchestrator/openvmm/node.py:894

• This PR is titled/described as adding virtio device support, but this change set also introduces shared_subnet TAP networking (including shared bridge lifecycle, dnsmasq reuse, and lease matching). Please update the PR description/title to reflect the additional feature scope, or consider splitting the shared_subnet work into a separate PR for easier review and rollback.

        if network.shared_subnet:

lisa/sut_orchestrator/openvmm/schema.py:317

• Defaulting smt to "off" will cause LISA to always pass an explicit SMT setting to OpenVMM (via --smt), which is a behavioral change for existing runbooks that previously relied on OpenVMM’s default. If the intent is to preserve prior defaults, consider using an empty default (meaning “don’t pass --smt unless configured”).

This issue also appears on line 369 of the same file.

    smt: str = OPENVMM_SMT_OFF

lisa/sut_orchestrator/openvmm/node.py:761

• vps_per_socket is being set to processor_count when the runbook leaves it unset. This forces every OpenVMM launch to pass --vps-per-socket, changing default CPU topology behavior even when users didn’t opt in. It’s safer to pass through runbook.vps_per_socket as-is (None) and let OpenVMM use its default unless explicitly configured.

This issue also appears on line 894 of the same file.

            vps_per_socket=(
                runbook.vps_per_socket
                if runbook.vps_per_socket is not None
                else processor_count
            ),

lisa/tools/openvmm.py:184

• This new exception message would be more actionable if it listed the supported values, so users can correct their runbook/config quickly.

            raise LisaException(
                f"Unsupported OpenVMM network device: {config.network_device}"
            )

selftests/test_openvmm_node.py:179

• The launch propagation test only asserts disk_device/network_device/network_queue_count, but the PR also added/propagated iommu, vps_per_socket, and smt into OpenVmmLaunchConfig. Adding assertions for these fields would prevent regressions in the newly introduced launch config wiring.

        launch_config = openvmm.launch_vm.call_args.args[0]
        self.assertEqual(OPENVMM_DISK_DEVICE_SCSI, launch_config.disk_device)
        self.assertEqual(OPENVMM_NETWORK_DEVICE_SYNTHETIC, launch_config.network_device)
        self.assertEqual(1, launch_config.network_queue_count)

lisa/tools/openvmm.py:177

• This new exception message doesn’t provide remediation guidance. Including the supported values (as is already done in the OpenVmm runbook schema validation) makes failures easier to diagnose and fix.

This issue also appears on line 182 of the same file.

            raise LisaException(
                f"Unsupported OpenVMM disk device: {config.disk_device}"
            )

lisa/tools/openvmm.py:197

• This new exception message is missing actionable guidance. Listing the supported IOMMU values (none/intel-vtd/amd-iommu) would align it with the schema validation and improve debuggability.

            raise LisaException(f"Unsupported OpenVMM IOMMU: {config.iommu}")

.github/workflows/continuous-integration-workflow.yml:73

• The PR description still contains the template placeholders (Description/Related Issue/Tests) and doesn’t document what was validated. Please fill in the description and test results so reviewers can confirm the behavior change safely.

      - name: Install Nox
        # argcomplete 3.7.1 evaluates PEP 604 unions at import time on Python 3.9.
        run: pip install nox toml "argcomplete!=3.7.1; python_version < '3.10'"

lisa/sut_orchestrator/openvmm/node.py:761

• vps_per_socket is treated as optional in OpenVmmLaunchConfig (only adds --vps-per-socket when not None), but launch() always sets it to processor_count when the runbook leaves it unset. That forces the CLI flag on every launch and may change OpenVMM’s default CPU topology behavior. Consider passing through the runbook value directly so the flag is only emitted when explicitly configured.

            vps_per_socket=(
                runbook.vps_per_socket
                if runbook.vps_per_socket is not None
                else processor_count
            ),

lisa/sut_orchestrator/openvmm/schema.py:320

• OpenVmmGuestNodeSchema defaults smt to off, and launch() always forwards runbook.smt, so OpenVmm.build_command() will always emit --smt off even when the runbook doesn’t specify SMT. If the intent is to preserve OpenVMM’s previous default behavior, consider making smt optional (e.g. empty/None) and only validating/emitting it when explicitly set.

            allow_none=True,
        ),
    )
    smt: str = OPENVMM_SMT_OFF
    min_raw_disk_size_gb: int = field(

lisa/tools/openvmm.py:197

• Error message for an unsupported IOMMU value doesn’t list the allowed options. Include the supported constants so users know how to fix the configuration.

            raise LisaException(f"Unsupported OpenVMM IOMMU: {config.iommu}")

lisa/tools/openvmm.py:184

• Error message for an unsupported network_device doesn’t list the allowed values, which makes misconfiguration harder to resolve. Include the supported device constants in the exception message.

            raise LisaException(
                f"Unsupported OpenVMM network device: {config.network_device}"
            )

lisa/tools/openvmm.py:177

• Error message for an unsupported disk_device doesn’t list the allowed values, which makes misconfiguration harder to diagnose. Include the supported device constants in the exception message.

This issue also appears in the following locations of the same file:

• line 182
• line 197

            raise LisaException(
                f"Unsupported OpenVMM disk device: {config.disk_device}"
            )

lisa/sut_orchestrator/openvmm/schema.py:123

• PR title/description focuses on virtio device selection, but this change also introduces additional OpenVMM runbook surface area (e.g., shared_subnet networking and CPU topology fields like vps_per_socket/smt). Please update the PR description/title to reflect the full scope, or split the unrelated behavior into a separate PR for easier review and rollback.

class OpenVmmNetworkSchema:
    mode: str = OPENVMM_NETWORK_MODE_USER
    device: str = OPENVMM_NETWORK_DEVICE_SYNTHETIC
    queue_count: Optional[int] = field(
        default=None,
        metadata=schema.field_metadata(
            field_function=schema.fields.Int,
            validate=schema.validate.Range(min=1, max=65535),
            allow_none=True,
        ),
    )
    shared_subnet: bool = False